fix: refine stability precedence to support standard prerelease sorting#6
Conversation
|
@joaopapereira Please review this follow-up change for #5 |
1 similar comment
|
@joaopapereira Please review this follow-up change for #5 |
sameerforge
force-pushed
the
topic/sameerforge/follow-up-semver-fix
branch
3 times, most recently
from
01fb27f to
d3217c1
Compare
|
@joaopapereira Please review this follow-up change for #5 |
There was a problem hiding this comment.
Pull request overview
This PR refines the metadata comparison logic in the semver library to support stability-aware precedence (GA > RC) in build metadata while preserving standard pre-release ordering. The changes focus on the versionExtension.Compare method, which now only applies stability-aware precedence when comparing numeric segments with alphanumeric segments containing pre-release markers (like "rc", "beta", etc.), rather than applying this logic uniformly to all string-vs-string comparisons.
Changes:
- Refined
isPreReleaseLabelfunction to require both a hyphen and a pre-release marker (alpha, beta, rc, pre, dev) to identify pre-release segments in numeric vs alphanumeric comparisons - Removed the string-vs-string pre-release comparison logic that was causing standard pre-release ordering (e.g., pre < rc) to be incorrectly evaluated
- Added four new test cases covering edge cases: vendir compatibility, numeric pre-release ordering, and natural sorting with build metadata
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| v4/semver.go | Refined the isPreReleaseLabel function to implement stability-aware precedence only for numeric vs alphanumeric comparisons, and removed problematic string-vs-string pre-release logic |
| v4/semver_test.go | Added four new edge-case test cases covering vendir compatibility, pre-release ordering, and natural sorting with build metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Sameer <sameer.khan@broadcom.com>
sameerforge
force-pushed
the
topic/sameerforge/follow-up-semver-fix
branch
from
d3217c1 to
750a47e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@joaopapereira PR is ready for your review now. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
v4/semver.go:510
- The logic for comparing alphanumeric segments no longer includes stability-aware precedence for pre-release markers. This breaks test case "3.4.0+v1.33-rc.2" vs "3.4.0+v1.33", where the build metadata segment "v1.33-rc" should be less than "v1.33" (GA should rank higher than RC). However, with the current code, naturalLess("v1.33-rc", "v1.33") returns false because "v1.33-rc" has more chunks than "v1.33" after the common prefix ["v", "1", ".", "33"], causing the comparison to return 1 instead of -1. The removed string-vs-string pre-release marker comparison needs to be reintroduced for alphanumeric segments containing pre-release identifiers, or the test expectation needs to be updated to reflect that RC is now considered greater than GA in build metadata.
if v.IsNum && !o.IsNum {
if isPreReleaseLabel(o.VersionStr) {
return 1 // Number wins against an RC/Alpha/Beta string
}
return -1 // Standard SemVer: Number < String
}
if !v.IsNum && o.IsNum {
if isPreReleaseLabel(v.VersionStr) {
return -1 // RC/Alpha/Beta string loses against a Number
}
return 1 // Standard SemVer: String > Number
}
// 3. Both are Alphanumeric
if v.VersionStr == o.VersionStr {
return 0
}
// Natural sort fallback for generic segments (e.g. "9-fips" vs "10-fips")
if naturalLess(v.VersionStr, o.VersionStr) {
return -1
}
return 1
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
All the Tests are passing as expected, along with the mentioned test case "3.4.0+v1.33-rc.2" vs "3.4.0+v1.33". The stability logic correctly prioritizes GA over RC before the natural sort fallback; this appears to be an AI false positive. |
|
@joaopapereira Please review the final changes. |
|
@joaopapereira Please review the changes. |
|
@joaopapereira Please review the changes. |
Refinement of Metadata Precedence Logic
This follow-up PR adjusts the build metadata comparison logic to ensure stability-aware precedence (GA > RC) only applies when comparing numeric segments to alphanumeric ones.
Verified Improvements:
Natural Sorting: vmware.10-fips now correctly ranks higher than vmware.9-fips.
Stability: 3.4.0+v1.33 (Stable) now correctly ranks higher than 3.4.0+v1.33-rc.2.
Compatibility: Standard pre-release ordering (e.g., 0.0.1-pre.1 < 0.0.1-rc.0) is preserved, fixing a regression found in vendir.
Validation :
All 38+ existing unit tests, alongside new edge-case coverage Passed ✅
Validation with Vendir Test :
Tests Passed ✅